-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Implement test_realloc #284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve git commit messages, addressing the motivation and considerations.
a867c69
to
d749753
Compare
b2ae174
to
0344bb8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't mention file name in the subject of git commit messages. Read https://cbea.ms/git-commit/ and CONTRIBUTING.md
carefully. Then, improve the commit messages by writing informative sentences.
0344bb8
to
ee8f240
Compare
harness.c
Outdated
|
||
const block_element_t *b = find_header(p); | ||
if (b->payload_size >= new_size) | ||
// TODO: Free some once we implement split. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Show more about this operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provide the improvements within this pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the current implementation has its pros and cons. Since we do not shrink the memory size, it performs well in cases like the following:
malloc(100) -> realloc(1) -> realloc(2) -> realloc(3)
In this scenario, when we allocate a large block of memory and then reduce its size, later extensions of the memory (to a larger size) will not require additional work to handle.
Here, I propose a simple improvement:
If new_size is larger than old_size, we allocate new_size * 2 for future use.
If new_size is smaller than old_size, we check whether new_size * 2 is still smaller than old_size. If so, we allocate new memory with new_size * 2 to optimize memory usage. Otherwise, we retain the old_size.
This approach wastes some memory but reduces the number of memory copy operations when calling realloc.
Apart from the above method, I also tried implementing it using the C function realloc, but the documentation does not guarantee under what circumstances it will move the pointer (allocate new memory). This uncertainty makes it difficult to implement a noallocate_mode.
Another approach is to split the unused memory into a separate block. However, this increases the complexity of implementing test_malloc and test_free. We would need an additional field in the block to indicate whether it is free or not. In test_malloc, the simplest implementation would iterate through the list to find a free block with enough space, which increases time complexity compared to the current implementation. In test_free, to avoid excessive fragmentation, we would need to implement a function to merge adjacent free memory blocks, adding further complexity.
I am unsure whether this is worth pursuing, as this module is primarily used for detecting memory usage rather than optimizing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am unsure whether this is worth pursuing, as this module is primarily used for detecting memory usage rather than optimizing it.
We can still take this pull request without touching the potential optimizations. Refine the commits to reflect this decision.
This commit implements test_realloc to support dynamic adjustment of memory sizes allocated by test_malloc or test_calloc. In this commit, when shrinking an allocated block (e.g., from 100 bytes to 10), memory corruption beyond the new size (such as m[i] for 10 <= i <= 99) cannot be detected. This limitation stems from the fact that the original memory block is retained after shrinking. While this approach helps avoid unnecessary memory copies, especially in cases where memory is often expanded and reduced, it can leave unused memory behind. Although this may improve performance when memory is plentiful, it may also lead to wasted space. There is still room for improvement in future versions to better manage memory use and detect corruption when shrinking blocks. Change-Id: Iebf68ea810c73549d966cb2305a5470863dacde5
ee8f240
to
a6678e3
Compare
Thank @chensheep for contributing! |
Added test_realloc implementation referencing https://danluu.com/malloc-tutorial/.
Change-Id: I87149ac233572dece7f3ec8b7d3320d07544f855